feat(@angular/ssr): redirect to preferred locale when accessing root route without a specified locale #29044

alan-agius4 · 2024-12-04T14:09:32Z

When users access the root route / without providing a locale, the application now redirects them to their preferred locale based on the Accept-Language header.

This enhancement leverages the user's browser preferences to determine the most appropriate locale, providing a seamless and personalized experience without requiring manual locale selection.

dgp1130

Left some initial thoughts. Accept-Language is specced, so I'd like to make sure we conform to that as closely as we can. Ideally it would be great if we could reuse some existing implementation, but I don't know if a good candidate exists.

packages/angular/build/src/utils/server-rendering/manifest.ts

packages/angular/ssr/src/i18n.ts

dgp1130 · 2024-12-06T01:55:15Z

packages/angular/ssr/src/i18n.ts

+  bestQuality = 0;
+  for (const { locale, quality } of parsedLocales) {
+    const currentQuality = quality ?? 1;
+    const languagePrefix = locale.split('-', 1)[0];


Bug: I don't think there's a limit of one - in a locale, we potentially need to handle at least 2 - characters and return the most specific locale.

Those are not used in the header. See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Language#directives

A language tag (which is sometimes referred to as a "locale identifier"). This consists of a 2-3 letter base language tag that indicates a language, optionally followed by additional subtags separated by -. The most common extra information is the country or region variant (like en-US or fr-CA) or the type of alphabet to use (like sr-Latn). Other variants, like the type of orthography (de-DE-1996), are usually not used in the context of this header.

usually not used

Is that a hard requirement of the spec? Does it limit to just a tag and a subtag?

I agree it's unlikely, I'm just aiming for compatibility with the spec here.

The spec mentioned that subtags meaning is not defined and it will result in unpredictable behaviour

https://datatracker.ietf.org/doc/html/rfc4647

The value and semantic meaning of private-use tags and of the subtags
used within such a language tag are not defined. Matching private-
use tags using language ranges or extended language ranges can result
in unpredictable content being returned.

Hmm, that does seem vague enough that we can mostly do what we want here. I'm ok to limit to only 2 locale segments for now if that makes the implementation easier. Should we consider throwing an error if an application defines a supported locale with 3 or more segments? Not sure if those would ever be matched.

I do kind of suspect that eventually someone will file a bug to support 3+ segments, but I'm fine to wait for that use case if/when it comes.

packages/angular/ssr/src/i18n.ts

dgp1130

Just responding to comments right now, I'll take another look at the code on Monday.

packages/angular/build/src/utils/server-rendering/manifest.ts

packages/angular/ssr/src/i18n.ts

dgp1130

This is looking pretty good, thanks for working through my long tail of compatibility questions.

I left a couple suggestions, mostly of the perf variety. I think the main thing which is missing the *;q=0.2 case, but otherwise no major concerns on my end.

packages/angular/ssr/src/i18n.ts

dgp1130 · 2024-12-09T23:53:16Z

packages/angular/ssr/src/i18n.ts

+  const normalizedSupportedLocales = new Map<string, string>();
+  for (const locale of supportedLocales) {
+    normalizedSupportedLocales.set(locale.toLowerCase(), locale);
+  }


Perf: Could we normalize the locales directly in the manifest? Why do we need to repeat this for each request?

If we can do this in the manifest, I definitely recommend defining a normalizeLocale function which does locale.toLowerCase() then call that for both the supported locales in the manifest as well as the Accept-Language locales. Having a single function normalize both makes it less likely they fall out of sync in the future and make it easier to track usage across the repo.

We could do this, but the improvement is so minimal that it doesn’t seem worthwhile.

For example, even in an extreme case where users have 200 locales and perform this operation over 1000 iterations, it only takes 0.018 ms.

dgp1130 · 2024-12-10T00:08:17Z

packages/angular/ssr/src/i18n.ts

+        bestMatch = normalizedSupportedLocales.get(supportedLocale);
+        break; // No need to continue searching for this locale.
+      }
+    }


Perf: This is a O(parsedLocales.length * normalizedSupportedLocales.size) check. I wonder if we could do something slightly more optimal by using a tree instead of a map, where each node in the tree is a locale segment. Such that you could have:

en -> US | > GB it es

Then looking up a locale is just walking the tree and if we don't find it, we use the deepest result we found which would inherently be the most specific locale.

If we limit this implementation to just two locale segments, it's probably not that important and likely fine to ignore for now. If we wanted to generically support N locale segments, it might be a useful optimization.

I'm not overly concerned about performance in this case, as the number of parsedLocales and normalizedSupportedLocales is usually quite small.

We could definitely consider this in the future.

dgp1130 · 2024-12-10T00:14:35Z

packages/angular/ssr/src/i18n.ts

+    if (!qualityZeroNormalizedLocales.has(normalizedLocale)) {
+      return locale;
+    }
+  }


Question: I've never really understood, what exactly are the semantics of q=0 anyways? Why would a browser send that? How is it different from just not sending a locale?

Based on this implementation, it seems like we're interpreting that to mean "I do not want this locale" and we therefore pick any other locale the user didn't explicitly reject. Is that the intended usage?

Yes, a quality value of 0 means the locale is explicitly not acceptable.

packages/angular/ssr/src/i18n.ts

dgp1130 · 2024-12-10T00:25:53Z

packages/angular/ssr/src/i18n.ts

+  bestQuality = 0;
+  for (const { locale, quality } of parsedLocales) {
+    const currentQuality = quality ?? 1;
+    const languagePrefix = locale.split('-', 1)[0];


Hmm, that does seem vague enough that we can mostly do what we want here. I'm ok to limit to only 2 locale segments for now if that makes the implementation easier. Should we consider throwing an error if an application defines a supported locale with 3 or more segments? Not sure if those would ever be matched.

I do kind of suspect that eventually someone will file a bug to support 3+ segments, but I'm fine to wait for that use case if/when it comes.

…route without a specified locale When users access the root route `/` without providing a locale, the application now redirects them to their preferred locale based on the `Accept-Language` header. This enhancement leverages the user's browser preferences to determine the most appropriate locale, providing a seamless and personalized experience without requiring manual locale selection.

angular-automatic-lock-bot · 2025-01-10T00:11:48Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

alan-agius4 added state: blocked target: minor This PR is targeted for the next minor release labels Dec 4, 2024

angular-robot bot added detected: feature PR contains a feature commit area: @angular/build area: @angular/ssr labels Dec 4, 2024

alan-agius4 force-pushed the locale-header branch 4 times, most recently from 0d5fe3e to 3d36469 Compare December 4, 2024 15:38

alan-agius4 requested a review from dgp1130 December 5, 2024 08:41

alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: blocked labels Dec 5, 2024

alan-agius4 marked this pull request as ready for review December 5, 2024 08:41

alan-agius4 force-pushed the locale-header branch 2 times, most recently from 7f38773 to 301803a Compare December 5, 2024 08:41

alan-agius4 removed the area: @angular/build label Dec 5, 2024

alan-agius4 force-pushed the locale-header branch 2 times, most recently from f0d9486 to 972c9a7 Compare December 5, 2024 15:13

dgp1130 reviewed Dec 6, 2024

View reviewed changes

alan-agius4 force-pushed the locale-header branch 7 times, most recently from b95081e to f941490 Compare December 6, 2024 13:29

alan-agius4 requested a review from dgp1130 December 6, 2024 13:46

dgp1130 reviewed Dec 7, 2024

View reviewed changes

alan-agius4 force-pushed the locale-header branch 2 times, most recently from 33d27cb to 26ee6d7 Compare December 9, 2024 07:22

dgp1130 approved these changes Dec 10, 2024

View reviewed changes

alan-agius4 force-pushed the locale-header branch from 26ee6d7 to b6651cc Compare December 10, 2024 11:40

alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Dec 10, 2024

alan-agius4 merged commit 41ece63 into angular:main Dec 10, 2024
31 checks passed

alan-agius4 deleted the locale-header branch December 10, 2024 12:11

angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 10, 2025

feat(@angular/ssr): redirect to preferred locale when accessing root route without a specified locale #29044

feat(@angular/ssr): redirect to preferred locale when accessing root route without a specified locale #29044

Uh oh!

Conversation

alan-agius4 commented Dec 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dgp1130 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgp1130 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dgp1130 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

angular-automatic-lock-bot bot commented Jan 10, 2025

Uh oh!

Uh oh!

alan-agius4 commented Dec 4, 2024 •

edited

Loading